-
Notifications
You must be signed in to change notification settings - Fork 495
Clang21 build fix #10998
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Clang21 build fix #10998
Conversation
First caught on Gentoo LLVM with Clang 21.1.5 and LLVM 21.1.5
Build with fail with the following error message
core/ucp_context.c:2673:47: error: default initialization of an object of type
'typeof (*tl_bitmap_super)' (aka 'const ucp_tl_bitmap_t') leaves the object uninitialized
[-Werror,-Wdefault-const-init-var-unsafe]
/var/tmp/portage/sys-cluster/ucx-1.19.0/work/ucx-1.19.0/src/ucs/datastruct/static_bitmap.h:250:5: note:
expanded from macro 'UCS_STATIC_BITMAP_NOT'
250 | UCS_STATIC_BITMAP_UNARY_OP(_bitmap, not, UCS_PP_UNIQUE_ID)
| ^
/var/tmp/portage/sys-cluster/ucx-1.19.0/work/ucx-1.19.0/src/ucs/datastruct/static_bitmap.h:239:5: note:
expanded from macro 'UCS_STATIC_BITMAP_UNARY_OP'
239 | _UCS_STATIC_BITMAP_UNARY_OP(_bitmap, _op_name, _uid)
| ^
/var/tmp/portage/sys-cluster/ucx-1.19.0/work/ucx-1.19.0/src/ucs/datastruct/static_bitmap.h:232:29: note:
expanded from macro '_UCS_STATIC_BITMAP_UNARY_OP'
232 | ucs_typeof(_bitmap) _r_##_uid; \
| ^
<scratch space>:54:1: note: expanded from here
54 | _r_0
| ^
core/ucp_context.c:2672:25: error: default initialization of an object of type
'typeof (*tl_bitmap)' (aka 'const ucp_tl_bitmap_t') leaves the object uninitialized
[-Werror,-Wdefault-const-init-var-unsafe]
2672 | ucp_tl_bitmap_t b = UCS_STATIC_BITMAP_AND(*tl_bitmap,
| ^
The cause is probably Clang now being more strict and ucs_typeof(_bitmap) _r_##_uid being declared and not used.
Signed-off-by: Brahmajit Das <[email protected]>
WalkthroughTwo bitmap macros in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/ucs/datastruct/static_bitmap.h(2 hunks)test/apps/iodemo/io_demo.cc(3 hunks)
🔇 Additional comments (4)
test/apps/iodemo/io_demo.cc (2)
23-23: LGTM!The
<random>header is correctly added to supportstd::random_deviceandstd::mt19937used in the C++17+ code path.
3003-3013: Good modernization for C++17 compatibility.The conditional compilation correctly addresses the removal of
std::random_shufflein C++17. The implementation properly usesstd::shufflewith a proper random number generator for C++17+ while maintaining backward compatibility.src/ucs/datastruct/static_bitmap.h (2)
229-237: LGTM!Zero-initializing
_r_##_uidproperly addresses Clang 21's-Wdefault-const-init-var-unsafewarning. The variable is immediately populated by the bitmap operation, so this initialization has no runtime impact beyond satisfying the compiler check.
254-265: LGTM!Zero-initializing
_r_##_uidproperly addresses Clang 21's-Wdefault-const-init-var-unsafewarning. The variable is immediately populated by the bitmap binary operation, so this initialization has no runtime impact beyond satisfying the compiler check.
56456ab to
5d2109e
Compare
random_shuffle was deprecated in C++14 and completely removed in C++17. With newer compilers like Clang 21, the build fails. This commit should preserve older behavior and use std::shuffle when available. Signed-off-by: Brahmajit Das <[email protected]>
5d2109e to
a950e7a
Compare
What?
Currently UCX is failing to build with newer compiler (for example Clang 21). This PR fixes that.
Why?
Clang 21 has become more strict. It now enables -Werror,-Wdefault-const-init-var-unsafe by default, which results in build failure.
Also std::random_shuffle has been removed since C++17 (deprecated in C++14). PR would make use of recommended std::shuffle and will fallback to std::random_shuffle if needed thus keeping old behavior intact.
How?
Not needed in this case I guess :)
Summary by CodeRabbit